Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

improve service support, + miscellaneous changes #32

Merged
merged 20 commits into from
Jul 7, 2024
Merged

improve service support, + miscellaneous changes #32

merged 20 commits into from
Jul 7, 2024

Conversation

mazunki
Copy link
Contributor

@mazunki mazunki commented Jun 28, 2024

  • write pidfile when running daemon
  • add logger info to pidfile
  • catch SIGHUP signal to reload config during runtime
  • use dist/ for packaging purposes
  • add openrc service
  • autoconf ignore
  • organize files, support autoconf
  • change some flags to subcommands
  • ignore wheels
  • change entry point to main
  • update documentation
  • update tests. permission denied claims 2==1?

@mazunki
Copy link
Contributor Author

mazunki commented Jun 28, 2024 •
edited
Loading

whoops, i didn't mean to request a merge for this yet, since it's a bit bundled together and out of order. was too quick on running github-cli while preparing it

i initially started working on adding support for pidfile because that makes service managers happy, and one thing led to another lol.

while i haven't changed any implementation behaviour, this PR is a breaking change. commands are no longer flags, for instance. i believe i have documented all the new things, though. if i missed something, please let me know.

i realize this PR would be best split into two different PRs, sorry about that. let me know if something should be changed :)

changes:

  • there is no longer a monolithic amdfan.py file, but rather a main module accessible through __main__.py or -m amdfan

  • core functionality is now split into subcommands, instead of exclusively relying on flags

  • distribution files like services and package builds are now under dist/. they are no longer copy and paste, and require the use of autotools (or sed). this should make it easier to adapt for niche users

  • documentation is now split into README and INSTALL, since end-users should really be using their package manager instead of manually installing system packages with pip

  • the daemon now writes a pidfile upon launch

  • the daemon now supports reloading config through a SIGHUP signal, without restarting the service entirely

  • we can now specify a ready notification file descriptor to be used

@mazunki mazunki changed the title main improve service support, + miscellaneous changes Jun 28, 2024
@mazunki
Copy link
Contributor Author

mazunki commented Jun 28, 2024

i think it would be wise to update and aggregate to the tests too. i only updated the tests which already existed

@mcgillij
Copy link
Owner

mcgillij commented Jul 6, 2024

Cool I'll take a look, sorry was out on vacation this week, so haven't kept up with GH.

mcgillij
mcgillij reviewed Jul 6, 2024
View reviewed changes
INSTALL.md


# Executable files
The executable files are contained within amdfan. This directory is a python module, and can either be loaded as `python -m amdgpu`. If you want to import the module, you may be interested in checking out `amdfan.commands`, which contains most of the subcommands.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
The executable files are contained within amdfan. This directory is a python module, and can either be loaded as `python -m amdgpu`. If you want to import the module, you may be interested in checking out `amdfan.commands`, which contains most of the subcommands.
The executable files are contained within amdfan. This directory is a python module, and can either be loaded as `python -m amdfan`. If you want to import the module, you may be interested in checking out `amdfan.commands`, which contains most of the subcommands.

@mcgillij
Copy link
Owner

mcgillij commented Jul 6, 2024 •
edited
Loading

Looks good, I'll have to pull it down to make sure I can still build it for the Arch packaging, and to make sure it still works with my py3status-amdfan, I'll likely just have to modify some of the commands on the widget side of things.

mcgillij added 5 commits July 7, 2024 03:21
@mcgillij
revert the PKGBUILD temporarily since it's blocking the build
00cf6ea 
and fix the broken tests, will have to redo how the commands are put
togther, however this will work for now.
@mcgillij
fix type error, and make mypy check the new source dir
1de3738
@mcgillij
remove python3.10 from the fix_tests_and_build
80683d8 
since typing.Self was introduced in 3.11 not going to deal with
backports
@mcgillij
Merge branch 'main' into main
81de4b4
@mcgillij
regenerate lock without python3.10
a774d49
@mcgillij mcgillij merged commit 1347791 into mcgillij:main Jul 7, 2024
2 checks passed
@mazunki
Copy link
Contributor Author

mazunki commented Jul 25, 2024

Oh, lovely, thanks for merging it. :)

Sorry about the delay myself, I have been AFK too. Is there anything left to do/change? I know the tests should probably be updated... but that's probably for another time.

It seems you got the PKGBUILD covered? If you think it's ready, I will bump the ebuild for Gentoo on ::guru too.

@mcgillij
Copy link
Owner

yeah I fixed at least the existing tests, so that the right cli funcs are passed so we no longer have the 2 == 1 etc.

The rest of the changes were just to make it work with the github workflow build.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mcgillij mcgillij mcgillij left review comments

Assignees

@mazunki mazunki

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@mazunki @mcgillij